Skip to content

fix(snapshot): per-slice merge + WireSnapshotState type at the wire boundary - #77

Merged
frenchie4111 merged 1 commit into
mainfrom
fix/snapshot-merge-per-slice
May 25, 2026
Merged

fix(snapshot): per-slice merge + WireSnapshotState type at the wire boundary#77
frenchie4111 merged 1 commit into
mainfrom
fix/snapshot-merge-per-slice

Conversation

@frenchie4111

Copy link
Copy Markdown
Collaborator

Summary

An Electron Harness client built after v2.9.3 crashes the renderer when it connects to a remote harness-server running v2.9.3 or older:

TypeError: Cannot read properties of undefined (reading 'find')
  at findCustom (useActiveTheme.ts)
  at resolveTheme
  at useActiveTheme

Repro. Commit 99262b2 (light/dark/system theme mode + custom themes from disk, 2026-05-22) added customThemes: CustomTheme[] to SettingsState. v2.9.3 was released the day before (2026-05-21), so its snapshot's settings object has no customThemes. The renderer's setSnapshot did a top-level shallow merge ({...initialState, ...state}) — that only protects against entirely-missing slices. Because the v2.9.3 server does send settings, the merge wholesale replaced initialState.settings (including the customThemes: [] default) with the partial wire object, and useActiveThemeresolveThemefindCustom(...customs.find) blew up on undefined.

The fix

  • Per-slice merge. New shared helper mergeWireSnapshot(state) in src/shared/state/index.ts walks every slice and merges { ...initialState[slice], ...state[slice] }. Fixes both skew cases — missing slice and missing field inside a slice — uniformly.
  • Type the wire boundary. New WireSnapshotState = { [K in keyof AppState]?: Partial<AppState[K]> } is what setSnapshot now accepts. Stops the wire boundary from lying about completeness. StateSnapshot.state stays as AppState because the sender (main process) genuinely produces a full shape; the structural-subtype path lets existing setSnapshot(snapshot.state) callers keep working without churn.
  • Bonus compile-time guard. mergeWireSnapshot returns AppState, so any future PR that adds a slice to AppState/initialState and forgets to add a line to the merge function gets a TypeScript build failure (the merge object literal won't satisfy AppState). Same forced-update story as adding to initialState.

Test plan

  • npm run typecheck
  • npx electron-vite build
  • npx vitest run src/shared src/renderer — 634 tests pass, including 4 new ones in src/shared/state/wire-merge.test.ts covering: (1) old server missing customThemes, (2) old server missing entire slice, (3) full snapshot pass-through, (4) explicit empty array from mid-version server.
  • Manual: launch Electron Harness from this branch, add a v2.9.3 remote backend, confirm the chip activates without the findCustom crash.

🤖 Generated with Claude Code

…oundary

A v2.9.3 server's snapshot has a `settings` object without `customThemes`
(commit 99262b2 added the field on 2026-05-22, the day after v2.9.3
shipped). The old top-level shallow merge only filled in entirely-
missing slices; because `state.settings` was present, it wholesale
replaced `initialState.settings` and the renderer crashed in
`useActiveTheme` → `resolveTheme` → `findCustom(...customs.find)` on
undefined.

Introduce a shared `mergeWireSnapshot(state)` helper that merges each
slice individually against `initialState`, and tighten `setSnapshot`'s
parameter type to `WireSnapshotState` (`{[K in keyof AppState]?:
Partial<AppState[K]>}`) so the wire boundary stops claiming snapshots
are always complete. The helper's `AppState` return type forces a
compile error if a future slice is added to `initialState` and the
merge function isn't updated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@frenchie4111
frenchie4111 force-pushed the fix/snapshot-merge-per-slice branch from d19ba03 to 6f7643a Compare May 25, 2026 17:32
@frenchie4111
frenchie4111 merged commit d1aa7b9 into main May 25, 2026
1 check passed
frenchie4111 added a commit that referenced this pull request May 25, 2026
The per-slice wire-merge introduced in #77 (d1aa7b9) enumerates every
AppState slice; the announcements slice that landed on this branch
needs the same merge line so the AppState object literal still
satisfies the type. TypeScript catches this by design — that's the
guard the new helper documents.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
frenchie4111 added a commit that referenced this pull request May 25, 2026
* feat(announcements): shared slice + settings fields

Adds an announcements slice (items + lastFetched + lastError) wired
into the root reducer alongside existing slices. Settings gets two
new fields, dismissedAnnouncementIds and announcementsMuted, with
events and tests, so the per-banner `×` and the "Hide all" action
can persist across reloads via the existing settings persistence
path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(announcements): main-process poller + persistence + IPC

Adds AnnouncementsPoller that fetches harness.mikelyons.org/announcements.json
on start and every 6h with a 10s AbortController timeout. Each entry is
validated strictly: id/title/href/publishedAt are required strings,
href must parse as an http(s) URL, and publishedAt must be a parseable
date — malformed entries are dropped individually with a debug-log
line, the rest of the feed is preserved. Network failures dispatch
fetchFailed and stay silent in the UI.

dismissedAnnouncementIds and announcementsMuted persist through the
existing config.json path so dismissals survive a reload. Three IPC
handlers (announcements:refresh / :dismiss / :mute) expose the writes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(announcements): renderer hook + banner UI

useAnnouncements() exposes the slice; the App-level useMemo filters
expired + dismissed entries, returns null when muted, otherwise picks
the entry with the newest publishedAt. The banner sits below the
update banners using the accent semantic color so it doesn't compete
with update green/info. Title links to href via shell.openExternal;
× dismisses just this id; the ⋯ menu offers "Hide all announcements"
which sets announcementsMuted. Window focus piggybacks an
announcements:refresh alongside the existing PR stale-refresh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(announcements): stretch menu wrapper so dropdown clears drag-region

The kebab button's wrapper was sized to the button (~22px), but the
banner row is ~42px tall with py-2.5 padding. items-center on the
row centered the wrapper vertically, leaving the absolute menu's
top edge inside the row — and the row's drag-region captured clicks
on the menu's top ~6px (no-drag isn't reliably inherited by
absolutely-positioned descendants in every Webkit build).

self-stretch + flex items-center stretches the wrapper to the full
row height while keeping the button visually centered, so top-full
now anchors the menu cleanly below the row. Also tagged the menu
container and the menuitem with no-drag as defense in depth.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(announcements): wire announcements slice into mergeWireSnapshot

The per-slice wire-merge introduced in #77 (d1aa7b9) enumerates every
AppState slice; the announcements slice that landed on this branch
needs the same merge line so the AppState object literal still
satisfies the type. TypeScript catches this by design — that's the
guard the new helper documents.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(site): empty announcements feed before banner ships

The welcome entry was test data for verifying the banner renders
end-to-end during development. Empty the array so the feature ships
with no banner showing by default — future announcements get added
deliberately, not because a placeholder slipped through.

The file stays in place (rather than getting deleted) so the URL
returns valid JSON and the poller's success path runs cleanly
instead of logging a 404 on every refresh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant